Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support copy/pasting between tabs and projects #10366

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Conversation

riknoll
Copy link
Member

@riknoll riknoll commented Jan 30, 2025

this adds support for copy/pasting code between tabs and projects. i borrowed the strategy of the cross tab copy paste plugin which stores copied data in localstorage instead of the clipboard (thanks for the pointer @srietkerk!). the reason we can't just use that plugin directly is because we have some extra error cases to deal with that the plugin doesn't handle:

first off, it's possible to copy code from one project that has an extension and paste a block from that extension into another project that does not have it installed. in this case, i display an error message explaining that the block can't be pasted. it might be good in the future to give information about what the exact missing extension is; right now i'm just checking to make sure all of the pasted blocks exist. we don't really have a way of associating a block with a given extension at the moment, but we could probably use the filename where the declaration is to figure it out.

secondly, you could copy code from a project in a different version of the editor (e.g. beta or maybe you just copied before the editor reload happened for a new version). 99% of the time that scenario is probably fine, but we do occasionally make changes to some of the blocks in the default extensions so i added a warning message that explains that pasting this code could harm your project. the "Paste Anyway" option on this dialog is in red to prevent people from mindlessly clicking it.

even with those warnings, there is still at least one other error case that i don't currently handle. it's possible that you could paste code from a project that has a different version of an extension where some of the blocks have changed. in this case, i think the most likely thing you'd see is some blocks that have some empty holes in them where the inputs have changed and you'd be able to just hit undo to fix it.

we could prevent that error altogether by enforcing that all installed extensions have to be the exact same version in both projects but i think that this scenario is unlikely enough that it would just end up being needlessly annoying for extensions that update frequently. maybe if we get to the point where we do actually associate the blocks with extensions then it might make sense to also add a check for this so that we can cut down on false positives

@riknoll riknoll requested a review from a team January 30, 2025 23:31
@srietkerk
Copy link
Contributor

Quick question, will this allow copy and pasting blocks across different targets? Like if someone tried to copy and paste blocks from micro:bit and put them in arcade, what would happen? I'm guessing that we would hit that first error case, but just wanted to ask.

@riknoll
Copy link
Member Author

riknoll commented Jan 30, 2025

@srietkerk nope. the targets are on different domains, so they can't access each other's localstorage. if we wanted to support that, then we would have to write to the clipboard instead but that's its own can of worms. clipboard support is better today than it used to be but can still be a little flaky

}

export function getCopyPasteHandlers() {
if (_handleCopy) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this check needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, they might be unset. the whole reason that this is in the external file is so that if our blockly code is required by a different part of the app that doesn't have the webapp code (like pxt renderer), everything will fall back to the default behavior

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Is it okay to just have the one check or should we check the other variables as well?

webapp/src/blocks.tsx Outdated Show resolved Hide resolved
webapp/src/blocks.tsx Outdated Show resolved Hide resolved
return !!!Blockly.clipboard.paste(copyData, copyWorkspace, centerCoords);
};

if (data.version !== 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding something, but how does the version given from the CopyDataEntry also provide an indicator for the version of the editor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we allow pasting from other versions of the editor, i just show a warning with a "paste anyway" option. this version number is meant to indicate a change in the copy data type which would prevent us from pasting at all even if the user chose to continue past the version mismatch

@srietkerk
Copy link
Contributor

@srietkerk nope. the targets are on different domains, so they can't access each other's localstorage. if we wanted to support that, then we would have to write to the clipboard instead but that's its own can of worms. clipboard support is better today than it used to be but can still be a little flaky

Ah, that makes sense!! I see that now in the code. Thanks for the clarification!

@riknoll riknoll merged commit 8bc2ef8 into master Jan 31, 2025
6 checks passed
@riknoll riknoll deleted the dev/riknoll/copy-paste branch January 31, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants